Skip to content

Fix/prevent double login in iframe#985

Draft
vaimdevs wants to merge 7 commits intomasterfrom
fix/prevent-double-login-in-iframe
Draft

Fix/prevent double login in iframe#985
vaimdevs wants to merge 7 commits intomasterfrom
fix/prevent-double-login-in-iframe

Conversation

@vaimdevs
Copy link
Copy Markdown
Contributor

@vaimdevs vaimdevs commented Mar 31, 2026

###Fix
- After data upload, exchange the user's JWT for a one-time token (OTT) via POST /api/v1/auth/jwt/ott/
- Append the OTT as ?token= in the iframe src URL
- Falls back gracefully if OTT exchange fails (logs warning, iframe still loads)

@vaimdev vaimdev requested a review from aucahuasi April 27, 2026 13:19
@aucahuasi
Copy link
Copy Markdown
Contributor

Thanks for this PR! Can you check these please:

Issue 1 (OTT is computed then thrown away)

The code never actually emits the OTT into the URL

graphistry/PlotterBase.py ~ lines 2301–2318:

url_params = dict(self._url_params)                # local copy
token = self.session.api_token
if token:
    try:
        ...
        url_params['token'] = resp.json()['ott']   # OTT goes into the local copy
    except Exception as e:
        logger.warning("Failed to exchange JWT for OTT: %s", e)

from graphistry.validate.validate_collections import normalize_collections_url_params
url_params = normalize_collections_url_params(self._url_params, ...)   # <-- whoops
viz_url = self._pygraphistry._viz_url(info, url_params)

That last line passes self._url_params (the original, no token) and clobbers the local dict you just put the OTT into. Net effect: every plot() does the OTT round-trip, throws the result away, the iframe URL has no ?token=, login screen still appears. Whole feature is dead-on-arrival.

The fix may be applying the OTT after normalize so nothing downstream can drop it, something like:

url_params = normalize_collections_url_params(self._url_params, validate=validate_mode, warn=warn)
token = self.session.api_token
if token:
    try:
        ...
        url_params['token'] = resp.json()['ott']
    except Exception as e:
        logger.warning("Failed to exchange JWT for OTT: %s", e)

Issue 2 (tests don't actually test the feature)

test_trace_headers_behavior.py only checks traceparent is on the OTT POST. Nothing checks ?token= lands in the returned URL. That's why Issue 1 wasn't caught.

I would suggest this addition:

url = g.plot(render="url", as_files=False, validate=False, warn=False, memoize=False)
assert "token=test-ott-token" in url

Other minor issues

  • No timeout on requests.post(...). If the OTT endpoint hangs, plot() hangs forever. Add timeout=..., match the rest of the upload path.
  • Logging on failure: bare except Exception is fine for graceful fallback, but please log resp.status_code + a snippet of the body when it's an HTTPError. Otherwise debugging an OTT outage in the field is painful.
  • Host choice: you're using self.session.protocol + self.session.hostname, while the iframe URL builder uses client_protocol_hostname(). Probably right for a server-to-server call (you want the real backend, not the proxy the browser sees), but worth a sanity check if those ever diverge in your deployments.

Copy link
Copy Markdown
Contributor

@aucahuasi aucahuasi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check:
#985 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants